-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Give Setup webview a single section #3448
Conversation
fc5aa0e
to
b8fd6ab
Compare
b8fd6ab
to
83c032e
Compare
} | ||
|
||
if (!hasData) { | ||
return <NoData /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid too many return
statements within this function.
} | ||
|
||
if (hasData === undefined) { | ||
return <EmptyState>Loading Project...</EmptyState> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid too many return
statements within this function.
return <NoData /> | ||
} | ||
|
||
return <EmptyState>{"You're all setup"}</EmptyState> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid too many return
statements within this function.
pythonBinPath: string | undefined | ||
} | ||
|
||
export const SetupExperiments: React.FC<SetupExperimentsProps> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function SetupExperiments
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
return <CliIncompatible checkCompatibility={checkCompatibility} /> | ||
} | ||
|
||
if (cliCompatible === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
@@ -8,3 +8,13 @@ export type SetupData = { | |||
projectInitialized: boolean | |||
pythonBinPath: string | undefined | |||
} | |||
|
|||
export enum Section { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this enum name unique (as well as the one in plot)? Importing after typing only section might be confusing if the wrong Section
is imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually got hit by this already. I will update after #3452.
Code Climate has analyzed commit 916e1a9 and detected 5 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 92.5% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.7% (0.1% change). View more on Code Climate. |
2/2
main
<- #3440 <- thisThis is exactly the same PR as #3441 which was merged in error. I have manually reverted that changed and replaced it with this PR.
This PR uses the shared component created in #3440 to give the Setup webview a single section.
Demo
Screen.Recording.2023-03-10.at.3.26.06.pm.mov
Note: This looks odd now but there will be a section above for the walkthrough and one below for Studio. I will also condense the
EmptyState
s toEmptySection
s (isFullScreen=false
)